Skip to content

fix(core): Mark package as side-effect free - #6829

Merged
antonis merged 2 commits into
getsentry:mainfrom
devclaimjuimperai:fix/side-effects-false
Oct 5, 2026
Merged

antonis merged 2 commits into
getsentry:mainfrom
devclaimjuimperai:fix/side-effects-false

Conversation

@devclaimjuimperai

Copy link
Copy Markdown
Contributor

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Adds "sideEffects": false to @sentry/react-native's package.json, same as @sentry/core, @sentry/react and @sentry/browser already have.

I grepped src/js for anything that runs at import time. The only top-level calls are in src/js/tools/ (metro config, babel transformer, collect-modules script), and those get loaded by Node directly, never bundled into the app. Nothing else patches globals on import.

💡 Motivation and Context

Fixes getsentry/sentry#126410. Without the flag, importing even just SDK_VERSION drags most of the SDK into the bundle with esbuild/Rollup/webpack (and Expo's tree shaking).

💚 How did you test it?

Ran the repro from the issue against the published 8.29.0, with and without the flag patched into its package.json (I also had to mark promise/* as external):

input files output bytes
before 121 87303
after 3 972

📝 Checklist

  • I added tests to verify changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.
  • No breaking changes.

🔮 Next steps

@devclaimjuimperai
devclaimjuimperai deleted the fix/side-effects-false branch October 3, 2026 22:41
@devclaimjuimperai
devclaimjuimperai restored the fix/side-effects-false branch October 3, 2026 23:16
@antonis antonis added the ready-to-merge Triggers the full CI test suite label Oct 5, 2026

@antonis antonis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉 Thank you for your contribution 🙇

Verified this is safe to mark side-effect free:

  • Tree-shaking confirmed — reproduced the issue's esbuild setup: import { SDK_VERSION } drops from ~86 KB / 119 input files down to a couple hundred bytes / 2 files with the flag applied. Matches the
    expected result in the issue.
  • No import-time side effects — audited everything reachable from index.ts (excluding Node-only tools/): no bare side-effect imports, no top-level global patching/polyfills/registration. The
    Promise/encode polyfills and all native/global touches run inside init()/wrap()/integration factories, so false (not a module allowlist) is the correct choice — consistent with @sentry/core,
    @sentry/browser and @sentry/react.
  • No API/behavior change — package metadata only; default Metro builds are unaffected, tree-shaking bundlers (Rollup/webpack/esbuild/Expo) get the win.

@antonis
antonis merged commit 49357a5 into getsentry:main Oct 5, 2026
102 of 112 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Triggers the full CI test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@sentry/react-native is not tree-shakeable: missing "sideEffects": false in package.json

2 participants